-
Notifications
You must be signed in to change notification settings - Fork 1.3k
update Vendor device to omit ep buffer for port with dedicated hwfifo #3373
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
hathach
commented
Nov 27, 2025
- update vendor device to omit ep buf when dedicated hwfifo is supported
- change tud_vendor_rx_cb() behavior, buffer and bufsize only available when CFG_TUD_VENDOR_RX_BUFSIZE = 0
- add tud_vendor_n_read_discard(), refactor vendor device. Update webusb example for more robust
- add tu_fifo_discard_n() and tu_edpt_stream_discard()
…b example for more robust
… when CFG_TUD_VENDOR_RX_BUFSIZE = 0 update vendor device to omit ep buf when dedicated hwfifo is supported
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
💡 Codex Review
Here are some automated review suggestions for this pull request.
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
This PR updates the vendor device implementation to support hardware endpoints with dedicated FIFO buffers, eliminating the need for local endpoint buffers in such configurations. This optimization reduces memory usage on hardware platforms with built-in FIFO support.
Key Changes:
- Modified stream read/write functions to handle both buffered and unbuffered (dedicated HWFIFO) modes
- Changed
tud_vendor_rx_cb()API behavior: buffer/bufsize parameters are only valid whenCFG_TUD_VENDOR_RX_BUFSIZE = 0 - Added
tu_fifo_discard_n()andtu_edpt_stream_discard()functions for discarding data without reading - Refactored vendor device structure and conditionally compiled endpoint buffers based on hardware capabilities
Reviewed changes
Copilot reviewed 8 out of 10 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| src/tusb.c | Updated stream write/read functions to support NULL ep_buf for dedicated HWFIFO mode; modified read behavior for non-FIFO mode |
| src/common/tusb_types.h | Added tusb_buffer_t struct definition |
| src/common/tusb_private.h | Added tu_edpt_stream_discard() inline helper function |
| src/common/tusb_fifo.h | Reorganized API sections and added tu_fifo_discard_n() declaration |
| src/common/tusb_fifo.c | Implemented tu_fifo_discard_n(), reorganized functions by API category, updated comments |
| src/class/vendor/vendor_device.h | Updated API signatures (itf→idx), added callback documentation, refactored function declarations |
| src/class/vendor/vendor_device.c | Restructured to conditionally compile endpoint buffers, refactored init logic, added find_vendor_itf() helper, updated callback invocation |
| src/class/cdc/cdc_device.c | Simplified variable declarations for consistency with vendor device changes |
| examples/device/webusb_serial/src/main.c | Refactored to use callback-driven approach instead of polling, removed cdc_task(), updated vendor RX callback to read from FIFO |
| examples/device/cdc_msc/src/main.c | Added DCD state toggle alongside DSR in uart notification |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
You can also share your feedback on Copilot code review for a chance to win a $100 gift card. Take the survey.
…SIZE > 0 vendor_write_flush() and write_available() only available when CFG_TUD_VENDOR_TX_BUFSIZE > 0
|
|
||
| tu_edpt_stream_read_xfer(rhport, &p_vendor->rx.stream); | ||
| } else if ( ep_addr == p_vendor->tx.stream.ep_addr ) { | ||
| tu_edpt_stream_read_xfer(rhport, &p_vendor->stream.rx); // prepare next data |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think it's useful to add a config whether next transfer is scheduled automatically or manually later in read_finished().
To give processing time for some commands.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yeah, we can have tud_vendor_configure() for manual read transfer including the intial xfer() when driver is open()
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I add CFG_TUD_VENDOR_RX_MANUAL_XFER (default to 0) instead of dynamic configure, since I think this is only helpful with no-fifo RX.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice, but I think the logic is inversed in vendord_open and vendord_xfer_cb ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ah right, sorry. It is late here already.
14cfcc4 to
84edff4
Compare
84edff4 to
6e9e9ce
Compare
|


